Update FEMC for new geometry: #1848

akioogawa · 2025-05-02T13:11:15Z

Briefly, what does this PR introduce?

Following Forward EM calorimeter geometry update:
eic/epic#855
this provides switch between Homogeneous and ScFi geometry implementations based on xml file loaded

For Homogenous, we keep energy smearing as is. SF=1.0
For ScFi, hits from fibers are summed to a tower and no enrgy smearing applied. SF=0.03.
Remove active insert which won't be built

Added an option to put SiPM saturation to CalorimeterHitDigi

Specify 2 new parameters : totalPixel and nPhotonPerGeV
default for totalPixel is 0, which case no attenuation is applied
For FEMC for both geometry models, SiPM saturation is ON by default
Use "-PFEMC:SiPMSaturation=OFF" to turn it off

See following links for some more details:
https://www.star.bnl.gov/~akio/epic/geometry/index.html
https://www.star.bnl.gov/~akio/epic/reco/index.html
https://www.star.bnl.gov/~akio/epic/hadron/index.html

What kind of change does this PR introduce?

Bug fix (issue #__)
New feature (issue #__)
Documentation update
Other: __

Please check if this PR fulfills the following:

Tests for the changes have been added
Documentation has been added / updated
Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

I don't think so, unless one provides wrong config xml file (until sum check in place)

Does this PR change default behavior?

It does apply SiPM attenuation by default for FEMC (but not other calorimeters).

Adding switch between Homogeneous and ScFi geometry implementations based on xml file loaded - For Homogenous, we keep enrgy smeasring as is. SF=1.0 - For ScFi, hits from fibers are summed to a tower and no enrgy smearing applied. SF=0.03. Added option to put SiPM saturation to CalorimeterHitDigi - Specify 2 new parameters : totalPixel and nPhotonPerGeV - default for totalPixel is 0, which case no attenuation is applied For FEMC for both geometry models, SiPM saturation is ON by default - Use "-PFEMC:SiPMSaturation=OFF" to turn it off

wdconinc · 2025-05-02T15:44:56Z

Please address the conflicts by performing a git rebase (preferably). As long as there are conflicts the automatic checks can't even start to run.

for more information, see https://pre-commit.ci

akioogawa · 2025-05-02T17:43:33Z

Done

akioogawa · 2025-05-05T19:53:52Z

As I understand, FEMC insert wil not be build. So it was removed from geometry and here FEMC factories. How can I remove it from auto checks?

wdconinc · 2025-05-06T03:36:44Z

As I understand, FEMC insert wil not be build. So it was removed from geometry and here FEMC factories. How can I remove it from auto checks?

Removing a detector would be something to do in a separate pr.

akioogawa · 2025-05-06T13:31:38Z

#1852

for more information, see https://pre-commit.ci

… to Homogeneous model (see https://www.star.bnl.gov/~akio/epic/reco/index.html) - Adding ScFi model's resolution smearing of 2.2% constant term based on high energy end (see https://www.star.bnl.gov/~akio/epic/reco/index.html) - Changing ADC threshold from 2 (~9MeV which was too low) to 3 (~15MeV as intended) - Rename an unfortunate variable name

for more information, see https://pre-commit.ci

src/algorithms/calorimetry/CalorimeterHitDigi.cc

src/detectors/FEMC/FEMC.cc

/home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:105:39: note: insert an explicit cast to silence this issue 105 | .numEffectiveSipmPixels = totalPixel}, | ^~~~~~~~~~ | static_cast<unsigned long long>( )

for more information, see https://pre-commit.ci

/home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:102:39: error: ISO C++ requires field designators to be specified in declaration order; field 'fields' will be initialized after field 'readoutType' [-Werror,-Wreorder-init-list] 102 | .readoutType = "sipm", | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~

``` /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:78:39: error: ISO C++ requires field designators to be specified in declaration order; field 'readout' will be initialized after field 'readoutType' [-Werror,-Wreorder-init-list] 78 | .readoutType = "sipm", | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~ /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:77:39: note: previous initialization for field 'readout' is here 77 | .readout = "EcalEndcapPHits", | ^~~~~~~~~~~~~~~~~ /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:100:39: error: ISO C++ requires field designators to be specified in declaration order; field 'readout' will be initialized after field 'readoutType' [-Werror,-Wreorder-init-list] 100 | .readoutType = "sipm", | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~ /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:99:39: note: previous initialization for field 'readout' is here 99 | .readout = "EcalEndcapPHits", | ^~~~~~~~~~~~~~~~~ /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:102:39: error: ISO C++ requires field designators to be specified in declaration order; field 'fields' will be initialized after field 'lightYield' [-Werror,-Wreorder-init-list] 102 | .lightYield = EcalEndcapP_nPhotonPerGeV / EcalEndcapP_PhotonCollectionEff, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:101:39: note: previous initialization for field 'fields' is here 101 | .fields = {"fiberx", "fibery", "x", "y"}, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 3 errors generated. ```

for more information, see https://pre-commit.ci

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/detectors/FEMC/FEMC.cc

This PR applies the include-what-you-use fixes as suggested by https://github.com/eic/EICrecon/actions/runs/18876404095. Please merge this PR into the branch `FEMCupdate20250501` to resolve failures in PR #1848. Auto-generated by [create-pull-request][1] [1]: https://github.com/peter-evans/create-pull-request Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

for more information, see https://pre-commit.ci

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/detectors/FEMC/FEMC.cc

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

This PR applies the include-what-you-use fixes as suggested by https://github.com/eic/EICrecon/actions/runs/18878525598. Please merge this PR into the branch `FEMCupdate20250501` to resolve failures in PR #1848. Auto-generated by [create-pull-request][1] [1]: https://github.com/peter-evans/create-pull-request Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

src/detectors/FEMC/FEMC.cc

No Clang-Tidy warnings found so I assume my comments were addressed

for more information, see https://pre-commit.ci

veprbl · 2025-10-28T15:50:26Z

JException
  Message:  JOmniFactory 'FEMC:EcalEndcapPRawHits': Wrong number of input collection names: 2 expected, 1 found.

Has this been tested at all?

akioogawa · 2025-10-28T19:56:54Z

Ran out of time yesterday. I'll test this Friday.

github-actions · 2025-10-28T22:02:45Z

Capybara summary for PR 1848

Last updated 2025-10-29T14:14-04:00 74dbeb0

github-actions bot added topic: calorimetry relates to calorimetry topic: forward topic: digitization labels May 2, 2025

akioogawa mentioned this pull request May 2, 2025

Femcal update eic/epic#855

Open

7 tasks

akioogawa and others added 2 commits May 2, 2025 13:42

Merge branch 'main' into FEMCupdate20250501

79ff346

[pre-commit.ci] auto fixes from pre-commit.com hooks

93febe8

for more information, see https://pre-commit.ci

typo

581c4b4

akioogawa temporarily deployed to github-pages May 2, 2025 19:32 — with GitHub Actions Inactive

Merge branch 'main' into FEMCupdate20250501

1d70ff4

akioogawa temporarily deployed to github-pages May 5, 2025 20:11 — with GitHub Actions Inactive

akioogawa and others added 2 commits May 8, 2025 09:05

Merge branch 'main' into FEMCupdate20250501

ee01baf

[pre-commit.ci] auto fixes from pre-commit.com hooks

3096e5e

for more information, see https://pre-commit.ci

pre-commit-ci bot temporarily deployed to github-pages May 8, 2025 14:03 Inactive

Merge branch 'main' into FEMCupdate20250501

3c04b82

akioogawa temporarily deployed to github-pages May 8, 2025 20:12 — with GitHub Actions Inactive

akioogawa and others added 4 commits May 12, 2025 09:23

Merge branch 'main' into FEMCupdate20250501

668fdf4

Merge branch 'main' into FEMCupdate20250501

e0e7b92

[pre-commit.ci] auto fixes from pre-commit.com hooks

1b7f3b7

for more information, see https://pre-commit.ci

pre-commit-ci bot temporarily deployed to github-pages May 13, 2025 17:34 Inactive

Merge branch 'main' into FEMCupdate20250501

32d7d30

akioogawa temporarily deployed to github-pages June 3, 2025 13:31 — with GitHub Actions Inactive

veprbl reviewed Jun 3, 2025

View reviewed changes

src/algorithms/calorimetry/CalorimeterHitDigi.cc Outdated Show resolved Hide resolved

veprbl reviewed Jun 3, 2025

View reviewed changes

src/detectors/FEMC/FEMC.cc Outdated Show resolved Hide resolved

veprbl and others added 7 commits October 27, 2025 21:17

Change totalPixel type to unsigned long long

80d23d2

/home/runner/work/EICrecon/EICrecon/src/detectors/FEMC/FEMC.cc:105:39: note: insert an explicit cast to silence this issue 105 | .numEffectiveSipmPixels = totalPixel}, | ^~~~~~~~~~ | static_cast<unsigned long long>( )

[pre-commit.ci] auto fixes from pre-commit.com hooks

956453f

for more information, see https://pre-commit.ci

[pre-commit.ci] auto fixes from pre-commit.com hooks

3b8fd8b

for more information, see https://pre-commit.ci

fields go last

4568907

Merge branch 'main' into FEMCupdate20250501

d71d25f

github-actions bot previously requested changes Oct 28, 2025

View reviewed changes

wdconinc mentioned this pull request Oct 28, 2025

Update FEMC for new geometry: (fix: iwyu) #2156

Merged

wdconinc and others added 3 commits October 28, 2025 10:33

address clang-tidy comments

0e7b743

[pre-commit.ci] auto fixes from pre-commit.com hooks

062f231

for more information, see https://pre-commit.ci

github-actions bot previously requested changes Oct 28, 2025

View reviewed changes

src/detectors/FEMC/FEMC.cc Outdated Show resolved Hide resolved

wdconinc mentioned this pull request Oct 28, 2025

Update FEMC for new geometry: (fix: iwyu) #2157

Merged

veprbl and others added 2 commits October 28, 2025 11:02

Update src/detectors/FEMC/FEMC.cc

1e93ae3

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

wdconinc reviewed Oct 28, 2025

View reviewed changes

src/detectors/FEMC/FEMC.cc Outdated Show resolved Hide resolved

veprbl and others added 2 commits October 28, 2025 11:16

Rename FEMCHomoScFi to EcalEndcapP_homogeniousFlag

2511b2f

[pre-commit.ci] auto fixes from pre-commit.com hooks

efef380

for more information, see https://pre-commit.ci

veprbl and others added 3 commits October 28, 2025 11:51

Update EcalEndcapPRawHits parameters in FEMC.cc

3c15a25

Fix typo in homogeneousFlag variable name

3249c9d

Merge branch 'main' into FEMCupdate20250501

0fafb40

akioogawa temporarily deployed to github-pages October 28, 2025 22:00 — with GitHub Actions Inactive

Merge branch 'main' into FEMCupdate20250501

74dbeb0

akioogawa temporarily deployed to github-pages October 29, 2025 18:14 — with GitHub Actions Inactive

Update FEMC for new geometry: #1848

Are you sure you want to change the base?

Update FEMC for new geometry: #1848

Conversation

akioogawa commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

Uh oh!

wdconinc commented May 2, 2025

Uh oh!

akioogawa commented May 2, 2025

Uh oh!

akioogawa commented May 5, 2025

Uh oh!

wdconinc commented May 6, 2025

Uh oh!

akioogawa commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

veprbl commented Oct 28, 2025

Uh oh!

akioogawa commented Oct 28, 2025

Uh oh!

github-actions bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Capybara summary for PR 1848

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

akioogawa commented May 2, 2025 •

edited

Loading

github-actions bot commented Oct 28, 2025 •

edited

Loading